-
Notifications
You must be signed in to change notification settings - Fork 146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Let client_secret to be optional for assertion grants #19
base: master
Are you sure you want to change the base?
Conversation
assertion grant. OAuth2's draft#10 state that client credentials must be validated only if present. See: http://tools.ietf.org/html/draft-ietf-oauth-v2-10#section-4.1.3
I'm not sure about this. The spec is slightly ambiguous about this; In any case, not requiring |
James Coglan said:
exactly! I have the same doubts you have.
You're right. I'm using Facebook and similar providers for my |
Certainly, if the secret is stored client-side, then you can't trust the identity of the client making the assertion, unless the assertion somehow contains the client's identity, which Facebook access tokens don't (although they do indicate a certain level of trust between the user and the client app). While the spec is not clear on this, and appears to make |
Looking at the last draft, they removed I cannot find any mention of |
Okay that sounds... weird. Let me find time to read it and see what I think. With your application, does it only exist client side, or does it run on confidential servers too? That is, does exposing the client_secret compromise the security of other software that would otherwise be secure? |
At the moment, exposing the client_secret would not lead to any security issue. But this sounds strange since it shoud be kept secret. |
It should be secret, but as the spec acknowledges, some clients cannot keep their credentials secret -- this is made explicit in later drafts with the For authorization, the |
James, per your gist (https://gist.github.com/3054344) it would seem clarification was made in later drafts that the client_id and client_secret need only be validated if they're both provided. Given that, do you anticipate merging this change? |
That gist refers to a different draft. In cases where draft-10 is ambiguous or undefined, I tend to refer to future drafts and/or common sense for security advice. I'm still not sure what to do about this. |
The spec actually says "validate the client credentials (if present)" for all request types. This seems like a misfeature, since if you allow an However you could imagine a client that operates in multiple places and therefore has its credentials exposed. If we allow credential-less exchanges then the client can authenticate properly in cases where its credentials are available, and not require those credentials to be deployed to unsafe environments. But this provides no incentive for the client to authenticate at all, and seems like a weird design. Anyone have any further thoughts on this? |
FWIW: I had interest in this initially because our clients were in javascript and mobile leading me to only "authenticate" the client based on the client_id. Since then we've moved to issuing per-client credentials (both an id and secret). This solved a number of things including allowing me to authenticate client identities more confidently. As mentioned earlier, if the assertion can also guarantee the client identity, this seems reasonable. Given that, this feels edge-casey to me. |
@jcoglan if you like I can re-post the PR to match the new code layout. Please, let me know if you're interested in getting it |
OAuth2's draft#10 state that client credentials must be verified for an assertion grant only if present.
http://tools.ietf.org/html/draft-ietf-oauth-v2-10#section-4.1.3
It is now possible to obtain an access token for a specific client via an assertion grant request without revealing the client's secret.